Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally Skip Push for "envify" #501

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Conversation

rience
Copy link
Contributor

@rience rience commented Sep 26, 2023

Pushing ".env" to remote host is not always required. If you just want to manage containers, traefik or accessories and initial push has already been done, that's adding additional step. Additionally - it reduces a risk of pushing incorrect ".env" to remote server.

This PR is adding "—skip_push" flag to "envify" command so it's possible to only generate it locally.

@northeastprince
Copy link
Contributor

I would be in favor of having this be the default behavior, but maybe having a flag like -p if you don't wanna run a seperate command - there's a lot less that can go wrong with being more explicit here.

@rience
Copy link
Contributor Author

rience commented Sep 30, 2023

You mean - default will be not doing push and then adding -p would do push?

@jmarchello
Copy link

I think the flag should be '--skip-push' (kabob case) to match the same flag from 'kamal deploy'.

@djmb
Copy link
Collaborator

djmb commented Oct 30, 2023

We push the env file by default because:

  1. That's one less step to get started
  2. Combining the two if safer because you are less likely to push a stale file if you've just generated it

But it makes sense to have this flag in cases where you don't want to push immediately.

@djmb djmb merged commit e9269d2 into basecamp:main Oct 30, 2023
6 checks passed
@rience rience deleted the optional-envify-push branch October 30, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants